HDDS-1802. Add Eviction policy for table cache.#1100
HDDS-1802. Add Eviction policy for table cache.#1100bharatviswa504 merged 9 commits intoapache:trunkfrom
Conversation
|
One missing thing in this PR is on the restart, we should load up tables with cache, where cleanup policy is NEVER from DB. Will post a fix for that in next commit. |
There was a problem hiding this comment.
I understand why we added this policy-specific check here. However it is probably misplaced here. Your original solution to have two cache types with isExist overloaded was probably better.
There was a problem hiding this comment.
Same here.. we should not have these checks here.
There was a problem hiding this comment.
The Cache doesn't know it is being invoked on flush.. so this check is misleading.
There was a problem hiding this comment.
It is caller responsibility to call this method after the flush. And right now this is called only after flushing to DB from OzoneManagerDoubleBuffer.
So, if it is NEVER, we will not do the cleanup.
This comment has been minimized.
This comment has been minimized.
|
Capturing our offline discussion. We discussed the following:
Anything I missed? |
Thank You @arp7 for the offline discussion. |
544f8fc to
89648ea
Compare
This comment has been minimized.
This comment has been minimized.
89648ea to
2dd7942
Compare
|
/retest |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@arp7 Thanks for the review. |
hadoop-hdds/common/src/main/java/org/apache/hadoop/utils/db/cache/CacheResult.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/utils/db/cache/TableCache.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/utils/db/cache/TableCacheImpl.java
Outdated
Show resolved
Hide resolved
arp7
left a comment
There was a problem hiding this comment.
+1
Minor suggestions to rename an enum and a function. Looks great otherwise.
|
Thank You @arp7 for the review. |
arp7
left a comment
There was a problem hiding this comment.
+1 pending CI. Thanks @bharatviswa504!
|
Thank You @arp7 for the review. |
|
💔 -1 overall
This message was automatically generated. |
… case sensitive. (apache#1100)
No description provided.